Handling central text overflow and adding tooltip for donut charts #26192

srmukher · 2023-01-06T08:25:14Z

Handling central text overflow and adding tooltip for donut charts.
Work item: 5321

Previous Behavior

Previous Behavior Example 1:

Previous Behavior Example 2:

New Behavior

New Behavior Example 1:

New Behavior Example 2:

Corner cases handled:

Case 1: Text is small : eg: "39,000" => No wrapping or truncation required

Case 2: Text is small but words are long (Single line). eg: "Incomprehensibilities" => Truncate horizontally, no wrapping required

Case 3: Text is small but words are long (Multi line) : "Tasks are incomprehensible" => Wrap and then truncate horizontally

Case 4: Text is long but words are small (Single line). eg: "It is done" => No wrapping or truncation required

Case 5: Text is long but words are small (Multi line) : eg: "Handling central text overflow and adding tooltip for donut charts" => After wrapping, if the text exceeds either horizontally (exceeds the central width) or vertically (if it goes beyond the inner circle vertically), then add the ellipsis (truncating) and break.

Case 6: Text and words both are long. eg: "Incomprehensibilities happen for texts that are not truncated" => After wrapping, if the text exceeds either horizontally (exceeds the central width) or vertically (if it goes beyond the inner circle vertically), then add the ellipsis (truncating) and break.

Case 7: Other languages (Japanese) single line: "やるべきことが残っている" => Truncate horizontally

Case 8: Other languages (Japanese) multi line: eg: "53 やるべきことが残っている" => Wrap and then Truncate horizontally

Related Issue(s)

https://uifabric.visualstudio.com/iss/_workitems/edit/5321/

Fixes #
Added text truncation for the central text of Donut chart when it overflows the inner radius of the Donut chart.
Added tooltip to show the entire text on hover, when the text inside the Donut chart is truncated.

…harts

fabricteam · 2023-01-06T08:34:53Z

📊 Bundle size report

_{🤖 This report was generated against d445789f33a50255a6a51548d7282a482bc78a0b}

size-auditor · 2023-01-06T08:36:50Z

Asset size changes

Size Auditor did not detect a change in bundle size for any component!

Baseline commit: d445789f33a50255a6a51548d7282a482bc78a0b (build)

srmukher · 2023-01-06T08:38:40Z

@microsoft-github-policy-service agree [company="Microsoft"]

srmukher · 2023-01-06T08:40:38Z

@microsoft-github-policy-service agree company="Microsoft"

codesandbox-ci · 2023-01-06T08:41:23Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 8ce67c1:

Sandbox	Source
@fluentui/react 8 starter	Configuration
@fluentui/react-components 9 starter	Configuration

AtishayMsft · 2023-01-13T12:56:08Z

packages/react-charting/src/components/DonutChart/Arc/Arc.tsx

@@ -80,6 +111,59 @@ export class Arc extends React.Component<IArcProps, IArcState> {
    wrapTextInsideDonut(classNames.insideDonutString, this.props.innerRadius! * 2 - TEXT_PADDING);
  }

+  private _getTruncatedText(text: string, maxWidth: number): string {


How does this work along with wrapTextInsideDonut, if wrapping and truncation both are enabled on the same text

wrapTextInsideDonut ensures that the words are placed in a new line if the current line length exceeds the maximum allowable width inside the donut. However, if a single word itself (even when placed in a new line) exceeds the maxWidth, it does not truncate the word. Thus, _getTruncatedText() determines if truncation is required for a single word or a group of words when inner text exceeds the maximum allowable width.

AtishayMsft · 2023-01-16T09:45:31Z

packages/react-charting/src/components/DonutChart/Arc/Arc.tsx

+            textAnchor={'middle'}
+            className={this._classNames.insideDonutString}
+            y={5}
+            id={'Donut_center_text'}


dont hardcode ids

Since the wrapping utility has dependency on this Id, need to keep this Id hardcoded

packages/react-charting/src/components/DonutChart/Arc/Arc.tsx

AtishayMsft · 2023-01-16T09:52:30Z

packages/react-charting/src/components/DonutChart/Arc/Arc.tsx

+
+      while ((word = words.pop()!)) {
+        line.push(word);
+        tspan.text(line.join(' ') + ' ');


You are truncating here without wrapping. First try to solve via wrapping and then by truncation. And lets discuss what all possible scenarios are there.

Made the change to wrap first and then truncate. Added 7 corner cases too that were handled.

In case 5 and case 6, you mentioned you are truncating vertically and horizontally, but you are truncating it only once. Can you clarify on this.

Atishay, this change is now outdated and I have moved it to the utilities.ts in wrapTextInsideDonut(). After wrapping, if the text exceeds either horizontally (exceeds the central width) or vertically (if it goes beyond the inner circle vertically), then we are adding the ellipsis and breaking. If we want to continue truncation even after the text is truncated one way, we will need to add ellipsis both horizontally and vertically might not be a good experience.

My comment is on the description that you have provided for case 5 and case 6. Here you are truncating only once and not both ways. Can you update the description.

Updated description.

…ers/srmukher/DonutBugFix

AtishayMsft · 2023-02-23T08:14:42Z

packages/react-charting/src/components/DonutChart/Arc/Arc.styles.ts

+      position: 'absolute',
+      textAlign: 'center',
+      top: '0px',
+      background: theme.semanticColors.bodyBackground,


is there a default theme always available

yes, the theme is coming as a prop from the parent component, which is a required property

packages/react-charting/src/components/DonutChart/Arc/Arc.tsx

AtishayMsft · 2023-02-23T08:42:45Z

packages/react-charting/src/utilities/utilities.ts

+        }
+        // Determine if truncation is required vertically
+        // If truncation is not required vertically, append a new line while taking care of horizontal truncation
+        if (tspan.node()!.getBoundingClientRect().y < maxWidth) {


shouldnt this be maxHeight

since maxHeight is the same as the maxWidth, I have reused maxWidth

AtishayMsft · 2023-02-23T08:48:41Z

packages/react-charting/src/utilities/utilities.ts

+          line = [word];
+          tspanEllipsis.text(word);
+
+          // Determine if truncation is appending the text exceeds maximum width vertically or horizontally


what do you mean by verticl width

I meant the vertical height, changing the sentence

…ers/srmukher/DonutBugFix

Hotell · 2023-04-17T09:42:06Z

CI issue discussed offline with @srmukher , this will unblock your pipeline #27584

AtishayMsft · 2023-06-13T10:24:32Z

packages/react-charting/src/components/DonutChart/DonutChart.base.tsx

+    this._tooltip = document.getElementById(this._tooltipId);
+    if (this._tooltip) {
+      this._tooltip!.innerHTML = text.toString();
+      this._tooltip!.style.display = 'block';


Can this be done using react states and not direct dom interactions

using react states require the title of the rect to be set in which case, there is no way to override the style of the tooltip as it appears as follows:

As a result, I have created a ref to the tooltip element as used it instead of querying the dom.

packages/react-charting/src/components/DonutChart/Arc/Arc.types.ts

packages/react-charting/src/utilities/utilities.ts

AtishayMsft · 2023-06-13T12:35:51Z

packages/react-charting/src/utilities/utilities.ts

+      // Determine if truncation is required vertically
+      // If yes, remove the last line, append ellipsis
+      // to the last word of the previous line (truncate horizontally) and break
+      if (tspan.node()!.getBoundingClientRect().y > maxWidth && line.length >= 1) {


this should be max height?

since maxHeight is the same as the maxWidth, I have reused maxWidth

AtishayMsft · 2023-06-13T12:42:41Z

packages/react-charting/src/utilities/utilities.ts

+  // Truncate till the text fits in the maximum allowable width
+  while (tspan.node()!.getComputedTextLength() > maxWidth - ellipsisLength) {
+    lastWord = line.pop()!;
+    if (lastWord !== undefined && lastWord.length >= truncateCount) {


shouldn't break happen only if last word is undefined. Is second part of the expression also relevant.

yes this is relevant because if the (tspan.node()!.getComputedTextLength() > maxWidth - ellipsisLength) and the lastword.length is < truncateCount, popping just the last word is sufficient after which it will append the ellipsis

AtishayMsft · 2023-08-16T15:47:18Z

packages/react-charting/src/components/DonutChart/DonutChart.base.tsx

@@ -118,11 +123,23 @@ export class DonutChartBase extends React.Component<IDonutChartProps, IDonutChar
      >
        <FocusZone direction={FocusZoneDirection.horizontal} handleTabKey={FocusZoneTabbableElements.all}>
          <div>
+            <div


Can we use SvgTooltipText control here.

AtishayMsft · 2023-08-16T15:48:00Z

packages/react-charting/src/components/DonutChart/Pie/Pie.tsx

@@ -80,6 +80,7 @@ export class Pie extends React.Component<IPieProps, {}> {
        {this.props.valueInsideDonut && (
          <text
            y={5}
+            id="Donut_center_text"


AtishayMsft · 2023-08-16T15:48:58Z

packages/react-charting/src/components/DonutChart/Arc/Arc.tsx

@@ -25,6 +26,10 @@ export class Arc extends React.Component<IArcProps, IArcState> {
    return null;
  }

+  public constructor(props: IArcProps) {
+    super(props);


Is this change needed?

AtishayMsft · 2023-08-16T15:51:05Z

packages/react-charting/src/utilities/utilities.ts

@@ -1351,11 +1351,12 @@ export function rotateXAxisLabels(rotateLabelProps: IRotateLabelProps) {
  return Math.floor(maxHeight / 1.414); // Compute maxHeight/tanInverse(45) to get the vertical height of labels.
 }

-export function wrapTextInsideDonut(selectorClass: string, maxWidth: number) {
+export function wrapTextInsideDonut(selectorClass: string, maxWidth: number, val: string) {


I don't think this val is needed as there is only 1 text element inside donut

AtishayMsft · 2023-08-16T16:00:46Z

packages/react-charting/src/utilities/utilities.ts

-          .text(word);
+        // append a line only if the line is within the max height
+        // else truncate horizontally and break
+        if (tspan.node()!.getBoundingClientRect().y + lineLength < maxWidth) {


How are you handling the case when text inside donut is "Is in is in in is in"

AtishayMsft · 2023-08-16T16:06:52Z

packages/react-charting/src/utilities/utilities.ts

-      if (tspan.node()!.getComputedTextLength() > maxWidth && line.length > 1) {
+      // Determine if wrapping is required
+      // If yes, append a new line only if it does not exceed the max height
+      // here, max width = max height, so maxWidth is reused to check max height


Does max height account for the curved shape. The available space for top line/bottom line is less than the middle line.
Or is this scenario too complex to handle.

AtishayMsft · 2024-01-02T11:26:48Z

@srmukher Reviving this PR. Could you merge master and update this PR for review.

Handling text overflow and adding tooltip for center texts of donut c…

7e3a3c2

…harts

srmukher requested a review from a team as a code owner January 6, 2023 08:25

msft-fluent-ui-bot added the Package: charting label Jan 6, 2023

msft-fluent-ui-bot assigned spmonahan Jan 6, 2023

AtishayMsft reviewed Jan 13, 2023

View reviewed changes

AtishayMsft reviewed Jan 16, 2023

View reviewed changes

packages/react-charting/src/components/DonutChart/Arc/Arc.tsx Outdated Show resolved Hide resolved

AtishayMsft reviewed Jan 16, 2023

View reviewed changes

Wrapping followed by truncation of inner text

42d2636

srmukher force-pushed the users/srmukher/DonutBugFix branch from c35dd98 to 42d2636 Compare February 2, 2023 07:07

srmukher added 3 commits February 6, 2023 11:26

Making the tooltip part of the donut chart svg instead of the body

322ca58

Adding interactive tests for dynamic donut chart inner text

62a5b7f

Merge branch 'master' of https://github.com/srmukher/fluentui into us…

d1d70a7

…ers/srmukher/DonutBugFix

AtishayMsft reviewed Feb 23, 2023

View reviewed changes

packages/react-charting/src/components/DonutChart/Arc/Arc.tsx Outdated Show resolved Hide resolved

AtishayMsft reviewed Feb 23, 2023

View reviewed changes

packages/react-charting/src/components/DonutChart/Arc/Arc.tsx Outdated Show resolved Hide resolved

AtishayMsft reviewed Feb 23, 2023

View reviewed changes

Resolving comments

66c1fa7

srmukher force-pushed the users/srmukher/DonutBugFix branch from 03d8453 to 66c1fa7 Compare February 23, 2023 10:28

srmukher added 2 commits April 13, 2023 11:54

Merging master

2457a45

Merge branch 'master' of https://github.com/srmukher/fluentui into us…

8ce67c1

…ers/srmukher/DonutBugFix

srmukher requested a review from a team as a code owner April 13, 2023 06:33

Hotell mentioned this pull request Apr 17, 2023

fix(scripts-tasks): avoid processing export declaration types in _evaluateImport #27584

Merged

Hotell removed the request for review from a team May 29, 2023 11:10

AtishayMsft reviewed Jun 13, 2023

View reviewed changes

packages/react-charting/src/components/DonutChart/Arc/Arc.types.ts Outdated Show resolved Hide resolved

AtishayMsft reviewed Jun 13, 2023

View reviewed changes

packages/react-charting/src/utilities/utilities.ts Show resolved Hide resolved

AtishayMsft reviewed Jun 13, 2023

View reviewed changes

srmukher added 2 commits August 3, 2023 11:54

Merging master

60789fd

Added few more changes

b9e2c85

AtishayMsft reviewed Aug 16, 2023

View reviewed changes

Handling central text overflow and adding tooltip for donut charts #26192

Are you sure you want to change the base?

Handling central text overflow and adding tooltip for donut charts #26192

Conversation

srmukher commented Jan 6, 2023 • edited Loading

Previous Behavior

New Behavior

Related Issue(s)

fabricteam commented Jan 6, 2023 • edited Loading

📊 Bundle size report

size-auditor bot commented Jan 6, 2023 • edited Loading

Asset size changes

srmukher commented Jan 6, 2023 via email • edited by msft-fluent-ui-bot Loading

srmukher commented Jan 6, 2023 via email • edited by msft-fluent-ui-bot Loading

codesandbox-ci bot commented Jan 6, 2023 • edited Loading

AtishayMsft Jan 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hotell commented Apr 17, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AtishayMsft commented Jan 2, 2024

srmukher commented Jan 6, 2023 •

edited

Loading

fabricteam commented Jan 6, 2023 •

edited

Loading

size-auditor bot commented Jan 6, 2023 •

edited

Loading

srmukher commented Jan 6, 2023 via email •

edited by msft-fluent-ui-bot

Loading

srmukher commented Jan 6, 2023 via email •

edited by msft-fluent-ui-bot

Loading

codesandbox-ci bot commented Jan 6, 2023 •

edited

Loading

AtishayMsft Jan 13, 2023 •

edited

Loading